Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Adds terms querying functionality for the CorePostTerms Block. #339

Merged
merged 12 commits into from
Feb 10, 2025
Merged

feat: Adds terms querying functionality for the CorePostTerms Block. #339

merged 12 commits into from
Feb 10, 2025

Conversation

whoami-pwd
Copy link
Contributor

@whoami-pwd whoami-pwd commented Feb 5, 2025 •
edited
Loading

Description

Related issue: #180

  • Adds terms querying functionality for the CorePostTerms Block.
    New terms field connection registered. Resolves as a list_of TermNode
    New taxonomy connection registered.

  • Additionally adds support for the prefix and suffix items with correspondent newly registered string fields.

query TestPostTerms($uri: String! = "test-terms") {
  nodeByUri(uri: $uri) {
    id
    uri
    ... on NodeWithPostEditorBlocks {
      editorBlocks {
        __typename
        ... on CorePostTerms {
          prefix
          suffix
          taxonomy {
            __typename
            node {
              __typename
              id
              name
            }
          }
          terms {
            __typename
            nodes {
              __typename
              id
              name
            }
          }
        }
      }
    }
  }
}

@whoami-pwd whoami-pwd requested a review from a team as a code owner February 5, 2025 15:08
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Feb 5, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: 03ef8c2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@wpengine/wp-graphql-content-blocks Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

justlevine
justlevine reviewed Feb 5, 2025
View reviewed changes
includes/Blocks/CorePostTerms.php Outdated
public function __construct( WP_Block_Type $block, Registry $block_registry ) {
parent::__construct( $block, $block_registry );

foreach ( self::STRING_FIELDS as $field ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like an unnecessary abstraction. I'd recommend using register_graphql_fields() (plural) directly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justlevine Good idea.
I’ve updated the code to remove the extra method call and directly register fields inside register_graphql_fields(). The abstraction has been minimized, but get_string_field_config() is still used for maintainability

@whoami-pwd whoami-pwd mentioned this pull request Feb 6, 2025
Closed
theodesp
theodesp previously approved these changes Feb 7, 2025
View reviewed changes
Copy link
Member

@theodesp theodesp left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!
@whoami-pwd it would be nice if we had a unit test to cover this query as well

@whoami-pwd whoami-pwd merged commit e935407 into main Feb 10, 2025
12 checks passed
@whoami-pwd whoami-pwd deleted the feature/180-core-post-terms-fetch-terms branch February 10, 2025 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justlevine justlevine justlevine left review comments

@theodesp theodesp theodesp approved these changes

@colinmurphy colinmurphy Awaiting requested review from colinmurphy

@moonmeister moonmeister Awaiting requested review from moonmeister

@josephfusco josephfusco Awaiting requested review from josephfusco

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@whoami-pwd @theodesp @justlevine